Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new move_rotator.py SAL Script #146

Merged
merged 2 commits into from
Oct 20, 2023
Merged

Add new move_rotator.py SAL Script #146

merged 2 commits into from
Oct 20, 2023

Conversation

b1quint
Copy link
Contributor

@b1quint b1quint commented Oct 12, 2023

Add the new move_rotator.py SAL Script. This depends on lsst-ts/ts_observatory_control#107.

Copy link
Member

@tribeiro tribeiro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, see my inline comment.

Copy link
Member

@tribeiro tribeiro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.. Take a look at my comments before merging please!


"""

def __init__(self, index: int, add_remotes: bool = True) -> None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can remove the add_remotes parameter now.

await self.mtcs.move_rotator(
angle=self.target_angle, wait_for_complete=self.wait_for_complete
)
if self.wait_for_complete:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I understand why you included this checkpoints here. Is the idea to provide a way for people to pause the script? I suggest you change it to be a single message, regardless of the value of self.wait_for_complete. Maybe something like:

await self.checkpoint(f"Move rotator finished.")

Or, if you want to provide information about the wait_for_complete, something like this:

await self.checkpoint(f"Move rotator finished; wait for complete: {self.wait_for_complete}.")

b1quint added a commit that referenced this pull request Oct 19, 2023
* Remove ``add_remotes`` argument from ``MoveRotator.__init__``
* Squeeze the checkpoints at the end to be a single line
b1quint added a commit that referenced this pull request Oct 20, 2023
* Remove ``add_remotes`` argument from ``MoveRotator.__init__``
* Squeeze the checkpoints at the end to be a single line
@b1quint b1quint force-pushed the tickets/DM-41081 branch 2 times, most recently from 85a1d00 to f850799 Compare October 20, 2023 14:56
@b1quint b1quint merged commit d4596e1 into develop Oct 20, 2023
2 checks passed
@b1quint b1quint deleted the tickets/DM-41081 branch October 20, 2023 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants